Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wgsl-in] Remove unused expressions from Module::const_expressions. #4648

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Nov 7, 2023

After building a Naga module from the WGSL AST, make a pass over the module to determine which expressions in Module::const_expressions are actually used, and compact out all unused expressions from that arena, adjusting references appropriately.

This isn't necessary on trunk, but it will be when we add abstract types to the WGSL front end. The evaluation of expressions containing abstract values will naturally introduce lots of 64-bit float and integer literals to the constant arena, which the validator would rightly reject. However, because all abstract values are converted to concrete types before appearing as part of the module's final code, all such 64-bit expressions should be unused.

Checklist

  • Run cargo clippy.
  • Run cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@jimblandy jimblandy requested a review from a team as a code owner November 7, 2023 21:11
@jimblandy jimblandy force-pushed the wgsl-in-compact-unused-const-expressions branch 2 times, most recently from 8f60bf8 to 7a0ac1f Compare November 7, 2023 21:15
@jimblandy
Copy link
Member Author

@teoxoy Since this isn't actually needed on trunk, should I put this on a separate branch in the repo, as we discussed doing for the override work?

After building a Naga module from the WGSL AST, make a pass over the
module to determine which expressions in `Module::const_expressions`
are actually used, and compact out all unused expressions from that
arena, adjusting references appropriately.

This isn't necessary on trunk, but it will be when we add abstract
types to the WGSL front end. The evaluation of expressions containing
abstract values will naturally introduce lots of 64-bit float and
integer literals to the constant arena, which the validator would
rightly reject. However, because all abstract values are converted to
concrete types before appearing as part of the module's final code,
all such 64-bit expressions should be unused.
@jimblandy jimblandy force-pushed the wgsl-in-compact-unused-const-expressions branch from 7a0ac1f to ba5292a Compare November 7, 2023 21:22
@jimblandy
Copy link
Member Author

After talking about this, @teoxoy pointed out that we are going to have to compact function expression arenas as well. Since it's harder to find the roots for those, it's actually probably not possible to be much simpler than the main compaction algorithm in crate::compact, and the WGSL front end should just call that.

The optimizations to avoid a stack / recursion I use in this PR could apply to the main compactor, though, and those should probably be applied.

@jimblandy jimblandy closed this Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant